Skip to content

Retake-photo confirmation, CI on dev and a cached API build - #61

Merged
aek676 merged 5 commits into
mainfrom
dev
Aug 31, 2026
Merged

aek676 merged 5 commits into
mainfrom
dev

Conversation

@aek676

@aek676 aek676 commented Aug 31, 2026

Copy link
Copy Markdown
Owner

Rolls #59 and #60 up to main. The content diff against main is exactly those two PRs — f7112ae and #56 are already on main through the #58 squash.

From #59 — confirm before retaking the receipt photo

  • Retaking deletes the session, so the receipt and every line item go with it. The action now sits behind an AlertDialog that says so, with an icon-sized trigger.
  • Drop capture="environment" from the file input, so the picker also offers the gallery.
  • Swap the arbitrary Tailwind values for scale tokens (max-w-160, max-w-80, border-l-3). No visual change.

From #60 — run the pipeline on dev, cache the API build

  • ci now runs on push and pull request for dev as well as main.
  • cd is guarded by github.ref == 'refs/heads/main', so dev builds and tests but never deploys.
  • publish.sh reuses a registry buildcache instead of rebuilding both platforms from scratch each run.
  • deploy/.env.example is trimmed to its variables.

Merging this is the first push to main that exercises the new cd guard, so the deploy job should run exactly as before.

aek676 added 4 commits August 30, 2026 18:59
* feat(deploy): ship the web to Cloudflare and the API to a VM

Wires the two halves of the pipeline that had no home yet. `cd.yml` gains a
deploy job: the web goes out with wrangler to a Worker serving static assets,
and the API is recreated over SSH on the VM that runs its image.

The API deploy pins the `sha-` tag rather than `latest`, writing it into the
VM's `.env` so a later `docker compose up` by hand brings back the same image
instead of drifting. The deploy key is restricted to a forced command on the
host, so it can recreate the API at a tag and nothing else.

`apps/api/deploy/` holds the templates the VM is set up from once, by hand;
a deploy never overwrites them.

* feat(infra): S3 receipts, infra and SSH scaffold sync (#57)

* feat(api): store receipts on S3-compatible storage, front the VM with Caddy

Replaces Google Cloud Storage with the S3 API, so the receipt images live in
Oracle Object Storage next to the VM that serves them. Every read goes through
the API (apps/api/src/modules/receipt), so same-region storage keeps that hop
inside Oracle instead of crossing the internet.

`Bun.S3Client` is native, which drops `@google-cloud/storage` without adding a
replacement, and the credentials become two env vars — no Application Default
Credentials to provision on a host that has no GCE metadata to read. The
variables are plain `S3_*`: nothing here is Oracle-specific, so MinIO serves
dev and tests, and moving to R2 later is an endpoint change.

Caddy now terminates TLS on the VM and proxies to the API over the compose
network, so the API stops publishing a port of its own.

`infra/` provisions what the deploy assumes exists: the bucket with the
retention rule that matches the session TTL, and an IAM user scoped to that one
bucket. The VM stays out of it deliberately — recreating an Always Free A1
shape is not something a plan should ever be allowed to propose.

* feat(infra): deliver VM scaffold via OCI CLI Run Command

Upload the API deploy files to the snapsplit-deploy bucket and push them to
the existing VM using the Oracle Cloud Agent Run Command, driven from a
null_resource that calls the oci CLI. The provider does not expose
InstanceAgentCommand as a resource, so the CLI is used instead. The step
skips cleanly on runners without the oci CLI (e.g. HCP Terraform) and
re-pushes whenever the deploy file contents change.

* debug(infra): expose deploy_root and deploy_files to diagnose empty scaffold

* chore(infra): remove temporary deploy debug outputs

* feat(deploy): sync scaffold via SSH forced-command wrapper

Add ssh-wrapper.sh as forced command (allows only sha-xxxxxxx,
mkdir -p, scp -t/sftp-server and chmod+chown+ls) and
sync-deploy-scaffold.sh plus cd.yml step to push
compose.yaml/deploy-on-host.sh/ssh-wrapper.sh/.env.example
before each deploy.

* refactor(deploy): delegate deploy to forced command

Simplify deploy-on-host.sh to read tag from SSH_ORIGINAL_COMMAND
and deploy.sh to exec ssh with the tag, delegating auth to the
forced command wrapper.

* chore(api): document DATABASE_NAME and clean test comment

* refactor(infra): remove deploy scaffold, now synced via CI

Drop the Terraform-managed deploy delivery (locals deploy_root/files/hash/script, oci_objectstorage_bucket.deploy, oci_objectstorage_object.deploy, oci_objectstorage_preauthrequest.deploy, time_static.par_expiry, null_resource.vm_scaffold, variable instance_ocid and output deploy_pull_commands). Scaffold is now synced by the CI SSH step (cd.yml Sync deploy scaffold to VM) via the forced-command wrapper, so the bucket/PARs and OCI CLI Run Command are obsolete. Remove null/time providers and lock entries.

* Revert "refactor(infra): remove deploy scaffold, now synced via CI"

This reverts commit edb07e0.

* Reapply "refactor(infra): remove deploy scaffold, now synced via CI"

This reverts commit bd483c3.

* refactor: rename infra/ to iac/

* refactor: strip redundant explanatory comments

* chore: remove docs/deployment.md and REVIEW.md
Bun's S3Client appends charset=utf-8 to text/* media types on stat().
Downstream consumers (extension lookup, Gemini API, HTTP headers) expect
bare MIME types, so split on ';' to normalize.
Build and test `dev` on push and PR, but keep the cd job pinned to
main so only main deploys. Reuse a registry buildcache in publish.sh
to avoid rebuilding both platforms from scratch on every run, and trim
the deploy .env.example down to its variables.
Replace the full-width retake button with an icon button behind an
alert dialog, since deleting the session discards the receipt and its
items. Also drop the `capture` attribute so the picker offers the
gallery too, and switch the arbitrary Tailwind values to scale tokens.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The registry buildcache added to publish.sh cannot produce a cache hit against the prebuilt target, and --cache-to defaults to ignore-error=false — which adds a way for CD to fail after the image has already been pushed.

Reviewed changes — main already carries a squash of most of dev (acc2651), so the 43-file diff GitHub renders is mostly content that is already live. I reviewed against the true net change (git diff-tree -r -p origin/main HEAD): 6 files, +57/-39.

  • CI on dev — push and pull_request now also fire for dev. Verified that no dev push, dev PR, or fork PR can reach the cd job or its inherited secrets: pull_request sets github.ref to refs/pull/N/merge, so event_name == 'push' && ref == 'refs/heads/main' excludes all three. nx-set-shas@v5 filters its last-successful-run lookup by mainBranchName, so dev runs cannot skew the affected range a later main push computes.
  • API image buildcache — publish.sh gains --cache-from/--cache-to against $image:buildcache. Flagged inline; the prebuilt target has no cacheable work.
  • apps/api/deploy/.env.example trimmed — all 18 comment lines removed. Flagged inline; nothing else in the repo documents these variables.
  • Retake photo behind a confirmation — the destructive delete is now correctly gated behind an AlertDialog, and the icon-only trigger does get an accessible name (Base UI's render merge lets the aria-label on AlertDialogTrigger win). One rough edge inline.
  • Tailwind arbitrary values → scale tokens — verified equivalent rather than assumed: built web and read the emitted CSS with --spacing: .25rem, giving max-w-160 = 640px, max-w-80 = 320px, border-l-3 = 3px. "No visual change" holds. biome format and biome lint apps/web are both clean on the changed files.
  • capture="environment" dropped — the picker now offers the gallery as well as the camera.

⚠️ Squash-merging this promotion widens the phantom diff for every future dev → main PR

The PR body has to explain that "the content diff against main is exactly those two PRs" — that explanation is itself the symptom. #58 was squash-merged, so the merge base between dev and main never advanced past 74e6f18, and this PR now replays #56 and f7112ae as 37 files of already-shipped infra. If #61 is squash-merged too, the merge base stays at 74e6f18 again and the next promotion replays all of this plus whatever is new. The noise compounds every cycle, and it is the kind of thing that hides a real change in a wall of green.

Technical details
# `dev` → `main` promotions need a strategy that advances the merge base

## Affected sites
- Repo merge strategy, not a specific line. `git merge-base origin/main dev` = `74e6f18`,
  while `main` is at `acc2651`. The 43-file / +828 diff is an artifact of that gap;
  `git diff-tree -r -p origin/main HEAD` shows the real 6-file change.

## Required outcome
- After each promotion, the merge base between `dev` and `main` advances, so the next
  `dev` → `main` PR shows only genuinely new work.

## Suggested approach (optional)
- Use a real merge commit for `dev` → `main` promotions (keep squash for topic branches
  into `dev`), which makes `main` an ancestor of the next `dev`.
- Or keep squashing and immediately reset/fast-forward `dev` to `main` after each
  promotion, so the branches re-converge.

## Open questions for the human
- Is `dev` intended to be a long-lived integration branch, or a staging branch that gets
  recreated from `main` after each release? The answer picks the strategy.
- Should branch protection on `main` restrict the allowed merge method to match?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment on lines +18 to +19
--cache-from "type=registry,ref=$image:buildcache" \
--cache-to "type=registry,ref=$image:buildcache,mode=max" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prebuilt target has nothing to cache. In apps/api/Dockerfile, prebuilt is FROM runtime (distroless/base-debian13) plus a single COPY dist/apps/api-linux/${TARGETARCH}/server — the deps and build stages aren't in that graph at all, because the cross-compile already happened outside Docker in the build:linux-amd64/build:linux-arm64 targets that publish depends on. The only non-base layer is the compiled binary, whose digest changes on exactly the commits that make api affected, so --cache-from can never hit; meanwhile --cache-to defaults to ignore-error=false, so a GHCR cache-export hiccup fails publish after the image has already pushed and blocks the deploy job that needs it.

Technical details
# Registry buildcache is unreachable for `--target prebuilt`

## Affected sites
- `apps/api/scripts/publish.sh:18-19` — `--cache-from` / `--cache-to type=registry,ref=$image:buildcache,mode=max`, applied to a `docker buildx build --target prebuilt`.
- `apps/api/Dockerfile` — `prebuilt` is `FROM runtime`, which is `FROM gcr.io/distroless/base-debian13:nonroot`. BuildKit only builds stages reachable from `--target`, so `deps` (`bun ci`) and `build` (`compile.sh`) are never instantiated.
- `apps/api/project.json` — `publish` `dependsOn` `build:linux-amd64` and `build:linux-arm64`, both nx-cached, both running `bun build --compile` on the host.

## Required outcome
- CD does not carry cache flags that cannot yield a hit, and a cache-export failure cannot fail a publish whose image push already succeeded.

## Suggested approach (optional)
- Simplest: drop both flags. The expensive work is already memoised by the nx cache on `build:linux-*`, and the real Docker build cost lives in the `hermetic` target, which `publish.sh` does not use.
- If you want to keep them as a hedge, at minimum set `ignore-error=true` on `--cache-to` and drop `mode=max` to `mode=min` — with no intermediate stages in the graph, `max` buys nothing over `min`.

## Open questions for the human
- Was the intent to speed up `hermetic` (which does run `bun ci` + `compile.sh` in-image) rather than `prebuilt`? If CI is meant to move to `hermetic`, the cache flags make sense there and this comment becomes a sequencing note instead.

@@ -0,0 +1,17 @@
API_IMAGE_TAG=latest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stripping the comments leaves this file undocumented anywhere in the repo — docs/deployment.md is gone, README.md has no deploy section, and docs/ now holds only dcu.puml and srs.md. This is the file a human copies to /opt/apps/snapsplit/.env and fills in by hand, and the removed lines carried precisely the non-obvious parts: that CORS_ORIGIN is the Cloudflare Worker origin rather than the API's own domain, why TRUST_PROXY=1 is required (without it the rate limiter keys every caller by Caddy's address and throttles them as one), and the terraform output commands that source the five S3_* values. Note apps/api/.env.example kept its comments, so the two files are now inconsistent.

Comment on lines +141 to +143
<AlertDialogCancel variant="ghost" size="xl">
Cancel
</AlertDialogCancel>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AlertDialogAction is disabled while the delete is in flight, but AlertDialogCancel isn't. Clicking Cancel in that window closes the dialog as though the retake were aborted, yet the DELETE is already in flight and onSettled still navigates to / a moment later with the session gone — a misleading signal on an irreversible action.

Base UI's AlertDialog already blocks outside-press dismissal (disablePointerDismissal = isAlertDialog || prop), but Escape still dismisses (escapeKey: isTopmost), so this only closes the button path. Fully sealing it would need a controlled open/onOpenChange guard, which is probably not worth the complexity here.

Suggested change
<AlertDialogCancel variant="ghost" size="xl">
Cancel
</AlertDialogCancel>
<AlertDialogCancel
variant="ghost"
size="xl"
disabled={deleteSession.isPending}
>
Cancel
</AlertDialogCancel>

# Conflicts:
#	apps/api/deploy/.env.example
@aek676
aek676 merged commit 13c6694 into main Aug 31, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant